Implement otel retry metrics #12064
Implement otel retry metrics #12064
Conversation
opentelemetry/src/main/java/io/grpc/opentelemetry/internal/OpenTelemetryConstants.java
Outdated
Show resolved
Hide resolved
opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java
Show resolved
Hide resolved
| @@ -64,8 +65,8 @@ public Stopwatch get() { | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
the gRFC has had recent updates, waiting for it to get merged before doing further changes...
There was a problem hiding this comment.
Yeah, there had been discussions yesterday about the recent changes to the retry metrics and whether they helped and such. The implementation should be mostly the same, but it isn't probably worth chasing the gRFC for the moment.
AgraVator
force-pushed
the
implement-otel-retry-metric
branch
from
a7a16ce to
3529769
Compare
AgraVator
force-pushed
the
implement-otel-retry-metric
branch
from
3529769 to
bd69ed5
Compare
| long hedgesPerCall = 0; | ||
| long attempts = hedgedAttemptsPerCall.get(); | ||
| if (attempts > 0) { | ||
| hedgesPerCall = attempts - 1; | ||
| } |
There was a problem hiding this comment.
should hedges be subtracted by 1 ?
There was a problem hiding this comment.
Hedged attempts don't include the original attempt so we shouldn't subtract.
| module.resource.clientCallDurationCounter() | ||
| .record(callLatencyNanos * SECONDS_PER_NANO, attribute); | ||
| module.resource.clientCallDurationCounter().record( | ||
| callLatencyNanos * SECONDS_PER_NANO, |
There was a problem hiding this comment.
callLatencyNanos is already in nanos.
|
|
||
| // Retry counts | ||
| if (module.resource.clientCallRetriesCounter() != null) { | ||
|
|
There was a problem hiding this comment.
nit: Delete empty line.
| } | ||
|
|
||
| if (retriesPerCall > 0) { | ||
| module.resource.clientCallRetriesCounter().record(retriesPerCall, baseAttributes); |
There was a problem hiding this comment.
Just use attemptsPerCall.get() - 1 > = 0 ? attemptsPerCall.get() - 1 : 0
without creating local variables?
| Optional<MetricData> metric = openTelemetryTesting.getMetrics().stream() | ||
| .filter(m -> m.getName().equals(metricName)) | ||
| .findFirst(); | ||
| if (metric.isPresent()) { |
There was a problem hiding this comment.
When will this metric not be present for the operations done by the tested on the tracer?
implements A96